feat: added sequencer set polling and consensus msgs queue #1144
Conversation
|
linter |
There was a problem hiding this comment.
Only did a quick scan, but might it be better to use an off shelf queue?
This is a ubiquitious data structure with numerous library implementations
e.g .https://github.com/liyue201/gostl?tab=readme-ov-file#queue
@danwt i think that's why it's easy to implement and maintain. external libraries introduce extra dependencies that we need to maintain. what if the library will become abandoned or archived? or if someday someone puts some malicious stuff in there and it goes unnoticed? plus usually these libs have a lot of overcomplicated logic that we don't need and won't ever use. in our case, we need a simple queue with very limited functionality, so i think it's not a problem to implement it ourselves using native go. |
keruch
changed the base branch from
kirill/1248-sequencer-reward-addr
to
main
| @@ -115,14 +115,9 @@ func (e *Executor) CreateBlock( | |||
| maxBlockDataSizeBytes = min(maxBlockDataSizeBytes, uint64(max(minBlockMaxBytes, state.ConsensusParams.Block.MaxBytes))) | |||
| mempoolTxs := e.mempool.ReapMaxBytesMaxGas(int64(maxBlockDataSizeBytes), state.ConsensusParams.Block.MaxGas) | |||
|
|
|||
| var consensusAnyMessages []*proto.Any | |||
| var consensusMsgs []proto2.Message | |||
| if e.consensusMessagesStream != nil { | |||
There was a problem hiding this comment.
in what situation is this nil? possibly redundnat validatoin?
| @@ -39,6 +37,15 @@ func (m *Manager) ProduceBlockLoop(ctx context.Context, bytesProducedC chan int) | |||
| select { | |||
| case <-ctx.Done(): | |||
| return nil | |||
|
|
|||
| case update := <-sequencerSetUpdates: | |||
There was a problem hiding this comment.
not sure about the location of this logic in the produce block loop.
thinking if there is a better place. maybe it's own goroutine.
| @@ -229,12 +226,18 @@ func (m *Manager) Start(ctx context.Context) error { | |||
| // channel to signal sequencer rotation started | |||
| rotateSequencerC := make(chan string, 1) | |||
|
|
|||
| // channel for sequencer set updates | |||
| sequencerSetUpdatesC := make(chan []types.Sequencer) | |||
There was a problem hiding this comment.
maybe we can move all this polling logic and channel initialization to a different method to not bloat the Start?
related to my comment that not sure why the sequecer updates signal is in the block production loop
| @@ -134,13 +197,36 @@ func (s *SequencerSet) GetByConsAddress(cons_addr []byte) *Sequencer { | |||
| return nil | |||
| } | |||
|
|
|||
| // SequencerListRightOuterJoin returns a set of sequencers that are in B but not in A. | |||
There was a problem hiding this comment.
| // SequencerListRightOuterJoin returns a set of sequencers that are in B but not in A. | |
| // SequencerListRightOuterJoin returns a set of sequencers that are in B but not in A. | |
| // sequencer is identified by a hash of all of it's fields. |
| @@ -134,13 +197,36 @@ func (s *SequencerSet) GetByConsAddress(cons_addr []byte) *Sequencer { | |||
| return nil | |||
| } | |||
|
|
|||
| // SequencerListRightOuterJoin returns a set of sequencers that are in B but not in A. | |||
| // CONTRACT: both A and B do not have duplicates! | |||
There was a problem hiding this comment.
not sure I follow this. why is this assumption necessary? worst case diff is empty no?
| // and is not thread-safe. Returns errors on serialization issues. | ||
| func (m *Manager) HandleSequencerSetUpdate(newSet []types.Sequencer) error { | ||
| // find new (updated) sequencers | ||
| newSequencers := types.SequencerListRightOuterJoin(m.State.Sequencers.Sequencers, newSet) |
There was a problem hiding this comment.
what if newSequencers is empty? probably worth returning early.
|
blocked by #1149 |
PR Standards
ADR links
Closes dymensionxyz/dymension#1248 and dymensionxyz/dymension#1226
For Author:
godoccommentsFor Reviewer:
After reviewer approval: